Skip to content

Suggestion: Add max_tries to transaction() #240

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Suggestion: Add max_tries to transaction() #240

wants to merge 1 commit into from

Conversation

@haukex
Copy link

@haukex haukex commented Nov 4, 2025 •
edited
Loading

Hi, I would like to propose that the transaction() helper gets an additional argument max_tries (perhaps even with a default value) to prevent infinite loops, as can happen fairly easily by accident:

vk = Valkey()
def incr(pipe: Pipeline):
    next_value = int(pipe.get('key')) + 1
    pipe.multi()
    vk.set('key', next_value)  # WRONG!
vk.transaction(incr, 'key', max_tries=100)  # would eventually raise error instead of infinite loop

If you like this change, I can also suggest some documentation updates etc. if you like.

@mkmkme
Copy link
Collaborator

mkmkme commented Nov 4, 2025

Hey,

Thanks for your contribution! Could you please sign off your commit? You can find how to do it here, in the failed check report.

@mkmkme
Copy link
Collaborator

mkmkme commented Nov 4, 2025

Also I would really appreciate if you could provide a test for your changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mkmkme mkmkme Awaiting requested review from mkmkme mkmkme is a code owner

@ahmedsobeh ahmedsobeh Awaiting requested review from ahmedsobeh ahmedsobeh is a code owner

@smeso smeso Awaiting requested review from smeso smeso is a code owner

@bogdanp05 bogdanp05 Awaiting requested review from bogdanp05 bogdanp05 is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@haukex @mkmkme